JS: fix npmPublish skip detection that always reported "already published"#7570
Merged
greg-at-moderne merged 2 commits intomainfrom May 5, 2026
Merged
JS: fix npmPublish skip detection that always reported "already published"#7570greg-at-moderne merged 2 commits intomainfrom
greg-at-moderne merged 2 commits intomainfrom
Conversation
…shed" PR #7552 added an `onlyIf` block that ran `npm view @openrewrite/rewrite@<version> version` and skipped publishing if the captured output contained the version string. With `redirectErrorStream(true)`, npm's 404 error message also contains the requested version (e.g. "The requested resource '@openrewrite/rewrite@<version>' could not be found"), so the contains-check matched on every miss and silently skipped npm publish for any newly-stamped snapshot. The npm prerelease tag has been frozen at 8.82.0-20260502-150813 since then, while Maven snapshots continue to advance with the latest commit timestamp. Downstream consumers that try to spawn the JS RPC subprocess via `npx --package=@openrewrite/rewrite@<jar-version> rewrite-rpc` then fail with ETARGET because the version baked into the JAR's resource is not on npm, breaking CI in repositories like rewrite-static-analysis. Switch the check to use `npm view`'s exit code (and verify the version appears on its own line in stdout) so we only skip when the version is genuinely already published.
1 task
Member
Author
|
@greg-at-moderne since you reviewed #7552 — small follow-up to fix the skip detection. Drafting until the next scheduled run lands so we can confirm npm and Maven versions re-align. |
| process.waitFor() | ||
| val output = process.inputStream.bufferedReader().readText().trim() | ||
| val alreadyPublished = output.contains(versionToCheck) | ||
| val alreadyPublished = process.waitFor() == 0 && output.lines().any { it.trim() == versionToCheck } |
Contributor
There was a problem hiding this comment.
Shouldn't it be contains instead of ==?
Member
Author
There was a problem hiding this comment.
Thanks! Applied just now; ready to merge if you agree (feel free to push the button as I'm out for a bit here).
Address review feedback: with the exit-code gate in place, the merged stderr/stdout 404 case can no longer reach this check, so a simple contains is enough and is more forgiving of minor npm view output formatting differences.
greg-at-moderne
approved these changes
May 5, 2026
github-project-automation
Bot
moved this from In Progress
to Ready to Review
in OpenRewrite
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
npm publishwhen snapshot version already exists #7552 added anonlyIfblock onnpmPublishthat runsnpm view @openrewrite/rewrite@<version> versionand skips publishing when the captured output contains the version string. WithredirectErrorStream(true), npm's 404 error message also contains the requested version:So
output.contains(versionToCheck)matches on every miss, and the daily snapshot publish has been silently skipped since 2026-05-02. Maven snapshots keep ticking forward with each new commit timestamp (currently at8.82.0-20260505-090457), but the npm prerelease tag is still frozen at8.82.0-20260502-150813.Downstream impact
Consumers spawn the JS RPC subprocess via:
where
<version-from-jar>is read fromMETA-INF/rewrite-javascript-version.txt. Once the npm/Maven versions diverge, that command fails withETARGETand the subprocess exits 1, surfacing in downstream tests asRPC process shut down early with exit code 1orExpected Content-Length header but received ''.This is breaking the rewrite-static-analysis scheduled CI on every full test run since 2026-05-04 (e.g. run 25335376932). Tests
AnnotateNullableMethodsTest.typescriptCodeandRemoveUnusedLocalVariablesTest$Typescript.noChangereproduce the failure locally on macOS once the localnpm linkis removed.Fix
Switch the skip check to use
npm view's exit code (and verify the version appears on its own line in stdout) so we only skip publishing when the version is genuinely already published. Drafting for review and to validate against the next scheduled CI run.Test plan
npm view @openrewrite/rewrite@<existing> versionand<nonexistent>and confirm the new logic agrees with reality